feat(JumpLinks): Animate scroll position based on selection#11800
feat(JumpLinks): Animate scroll position based on selection#11800rebeccaalpert wants to merge 1 commit intopatternfly:mainfrom
Conversation
|
Preview: https://patternfly-react-pr-11800.surge.sh A11y report: https://patternfly-react-pr-11800-a11y.surge.sh |
rebeccaalpert
requested review from
a team,
kmcfaul,
srambach and
wise-king-sullyman
and removed request for
a team
|
I see the same behavior that I struggled with - if you click a link low on the page, and another above it but also low on the page, you can see that instead of smoothly scrolling up to the second link, it sort of jumps up further before sliding down a bit. Also, it doesn't seem to respect my prefers-reduced-motion setting - without that I'd definitely hesitate to make this change. Here's an article that talks about how to use matchMedia to do this. |
kmcfaul
left a comment
kmcfaul
left a comment
There was a problem hiding this comment.
I've also noticed the scroll behavior is odd when on a lower item on the page and click a jump link to get higher on the page.
The code looks sounds but unsure if we want to let this go in with the animation quirks.
|
Agree with the comments above and thanks for tackling this issue @rebeccaalpert. The jumping back to the top before scrolling back down seems a bit odd. If there isn't a medium/easy fix I think we can hold this PR for the time being. There are other issues we can focus on. |
|
Sounds good. I don't mind noodling a little bit more this week, but if I can't get it to play along I will close this. |
5 participants
Refactored this to be able to handle smooth scrolling - had to drop prior logic that adjusted active item in scroll so scrolling doesn't update the active item. I did this to match main's behavior, but let me know if you want that as a fun extra. The targetTop logic is there to accommodate the last item in a list, which may not have sufficient scroll padding.
I'm less familiar with this component, so I would appreciate if someone could give this a look-over and let me know if I broke something that's less obvious.
What: Closes 11763
Additional issues: